automatic removing of invalid spaces in file names

to ensure compatibility with Widnows, we will remove automatically the
leading space characters in file name of new files or folders

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 75abf7ca4381888a577547831f4da82ac678661b..646808147e3b1e3d735fb423da7371294fb4c79f 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -420,14 +420,17 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent
         case CSYNC_FILE_EXCLUDE_TRAILING_SPACE:
             item->_errorString = tr("Filename contains trailing spaces.");
             item->_status = SyncFileItem::FileNameInvalid;
+            maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
             break;
         case CSYNC_FILE_EXCLUDE_LEADING_SPACE:
             item->_errorString = tr("Filename contains leading spaces.");
             item->_status = SyncFileItem::FileNameInvalid;
+            maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
             break;
         case CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE:
             item->_errorString = tr("Filename contains leading and trailing spaces.");
             item->_status = SyncFileItem::FileNameInvalid;
+            maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
             break;
         case CSYNC_FILE_EXCLUDE_LONG_FILENAME:
             item->_errorString = tr("Filename is too long.");
@@ -2274,4 +2277,38 @@ void ProcessDirectoryJob::setupDbPinStateActions(SyncJournalFileRecord &record)
     }
 }
 
+void ProcessDirectoryJob::maybeRenameForWindowsCompatibility(const QString &absoluteFileName,
+                                                             CSYNC_EXCLUDE_TYPE excludeReason)
+{
+    if (!_discoveryData->_shouldEnforceWindowsFileNameCompatibility) {
+        return;
+    }
+
+    const auto fileInfo = QFileInfo{absoluteFileName};
+    switch (excludeReason)
+    {
+    case CSYNC_NOT_EXCLUDED:
+    case CSYNC_FILE_EXCLUDE_CASE_CLASH_CONFLICT:
+    case CSYNC_FILE_EXCLUDE_AND_REMOVE:
+    case CSYNC_FILE_EXCLUDE_CANNOT_ENCODE:
+    case CSYNC_FILE_EXCLUDE_INVALID_CHAR:
+    case CSYNC_FILE_EXCLUDE_SERVER_BLACKLISTED:
+    case CSYNC_FILE_EXCLUDE_HIDDEN:
+    case CSYNC_FILE_SILENTLY_EXCLUDED:
+    case CSYNC_FILE_EXCLUDE_STAT_FAILED:
+    case CSYNC_FILE_EXCLUDE_LONG_FILENAME:
+    case CSYNC_FILE_EXCLUDE_LIST:
+    case CSYNC_FILE_EXCLUDE_CONFLICT:
+        break;
+    case CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE:
+    case CSYNC_FILE_EXCLUDE_LEADING_SPACE:
+    case CSYNC_FILE_EXCLUDE_TRAILING_SPACE:
+    {
+        const auto renameTarget = QString{fileInfo.absolutePath() + QStringLiteral("/") + fileInfo.fileName().trimmed()};
+        FileSystem::rename(absoluteFileName, renameTarget);
+        break;
+    }
+    }
+}
+
 }

diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h
index 840eb15d61169e2d6d071a7893fd41cb4fbe6075..d87114d8bd8194edb2c71b1349d6e8e8cbe0902c 100644 (file)
--- a/src/libsync/discovery.h
+++ b/src/libsync/discovery.h
@@ -15,6 +15,7 @@
 #pragma once
 
 #include <QObject>
+#include "csync_exclude.h"
 #include "discoveryphase.h"
 #include "syncfileitem.h"
 #include "common/asserts.h"
@@ -255,6 +256,9 @@ private:
      */
     void setupDbPinStateActions(SyncJournalFileRecord &record);
 
+    void maybeRenameForWindowsCompatibility(const QString &absoluteFileName,
+                                            CSYNC_EXCLUDE_TYPE excludeReason);
+
     qint64 _lastSyncTimestamp = 0;
 
     QueryMode _queryServer = QueryMode::NormalQuery;

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
index f1d1d3136dd654135ab60623decf08e2649d72d9..3382ffc1315e7a2aab353534f3e3cf139bff22f8 100644 (file)
--- a/src/libsync/discoveryphase.h
+++ b/src/libsync/discoveryphase.h
@@ -326,6 +326,7 @@ public:
     QRegularExpression _invalidFilenameRx; // FIXME: maybe move in ExcludedFiles
     QStringList _serverBlacklistedFiles; // The blacklist from the capabilities
     QStringList _leadingAndTrailingSpacesFilesAllowed;
+    bool _shouldEnforceWindowsFileNameCompatibility = false;
     bool _ignoreHiddenFiles = false;
     std::function<bool(const QString &)> _shouldDiscoverLocaly;
 

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 2ac90bf1e8df3b0d0124bcb689bb10cfe7389188..d5018149b31c82a7279abeaeb84b591d5a048c79 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -647,6 +647,7 @@ void SyncEngine::startSync()
 
     _discoveryPhase = std::make_unique<DiscoveryPhase>();
     _discoveryPhase->_leadingAndTrailingSpacesFilesAllowed = _leadingAndTrailingSpacesFilesAllowed;
+    _discoveryPhase->_shouldEnforceWindowsFileNameCompatibility = _shouldEnforceWindowsFileNameCompatibility;
     _discoveryPhase->_account = _account;
     _discoveryPhase->_excludes = _excludedFiles.data();
     const QString excludeFilePath = _localPath + QStringLiteral(".sync-exclude.lst");
@@ -1208,6 +1209,11 @@ void SyncEngine::addAcceptedInvalidFileName(const QString& filePath)
     _leadingAndTrailingSpacesFilesAllowed.append(filePath);
 }
 
+void SyncEngine::setLocalDiscoveryEnforceWindowsFileNameCompatibility(bool value)
+{
+    _shouldEnforceWindowsFileNameCompatibility = value;
+}
+
 bool SyncEngine::wasFileTouched(const QString &fn) const
 {
     // Start from the end (most recent) and look for our path. Check the time just in case.

diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index 2dc5bfa6b4a322f8ad19c9710d6bf44df29881bc..76fc99ce7dc54c7c2c540255ca2598ff3b44127b 100644 (file)
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -159,6 +159,7 @@ public slots:
      */
     void setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle style, std::set<QString> paths = {});
     void addAcceptedInvalidFileName(const QString& filePath);
+    void setLocalDiscoveryEnforceWindowsFileNameCompatibility(bool value);
 
 signals:
     // During update, before reconcile
@@ -409,6 +410,7 @@ private:
     std::set<QString> _localDiscoveryPaths;
 
     QStringList _leadingAndTrailingSpacesFilesAllowed;
+    bool _shouldEnforceWindowsFileNameCompatibility = false;
 
     // Hash of files we have scheduled for later sync runs, along with a
     // pointer to the timer which will trigger the sync run for it.

diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index cecd9ae59f65d0e3e0c55013180fdbc7faefcbcf..1a1d55971bc67abeb9e2ef0201c60c458acc7385 100644 (file)
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -2035,6 +2035,78 @@ private slots:
 
         QVERIFY(fakeFolder.syncOnce());
     }
+
+    void testCreateFileWithTrailingLeadingSpaces_local_automatedRenameBeforeUpload()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+        fakeFolder.syncEngine().setLocalDiscoveryEnforceWindowsFileNameCompatibility(true);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        const QString fileWithSpaces1(" foo");
+        const QString fileWithSpaces2(" bar ");
+        const QString fileWithSpaces3("bla ");
+        const QString fileWithSpaces4("A/ foo");
+        const QString fileWithSpaces5("A/ bar ");
+        const QString fileWithSpaces6("A/bla ");
+        const auto extraFileNameWithSpaces = QStringLiteral(" with spaces ");
+        const QString fileWithoutSpaces1("foo");
+        const QString fileWithoutSpaces2("bar");
+        const QString fileWithoutSpaces3("bla");
+        const QString fileWithoutSpaces4("A/foo");
+        const QString fileWithoutSpaces5("A/bar");
+        const QString fileWithoutSpaces6("A/bla");
+        const auto extraFileNameWithoutSpaces = QStringLiteral("with spaces");
+
+        fakeFolder.localModifier().insert(fileWithSpaces1);
+        fakeFolder.localModifier().insert(fileWithSpaces2);
+        fakeFolder.localModifier().insert(fileWithSpaces3);
+        fakeFolder.localModifier().mkdir("A");
+        fakeFolder.localModifier().insert(fileWithSpaces4);
+        fakeFolder.localModifier().insert(fileWithSpaces5);
+        fakeFolder.localModifier().insert(fileWithSpaces6);
+        fakeFolder.localModifier().mkdir(extraFileNameWithSpaces);
+
+        ItemCompletedSpy completeSpy(fakeFolder);
+        completeSpy.clear();
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces1)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces2)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces3)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces4)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces5)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces6)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithoutSpaces)->_status, SyncFileItem::Status::NoStatus);
+
+        completeSpy.clear();
+
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, {QStringLiteral("foo"), QStringLiteral("bar"), QStringLiteral("bla"), QStringLiteral("A/foo"), QStringLiteral("A/bar"), QStringLiteral("A/bla")});
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::NoStatus);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces1)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces2)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces3)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces4)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces5)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(fileWithoutSpaces6)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithoutSpaces)->_status, SyncFileItem::Status::Success);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncEngine)